Skip to content

fix(routing): prevent dynamic static-suffix routes from shadowing static routes#6

Open
JaredStowell wants to merge 1 commit intomainfrom
codex/fix-route-precedence-regression-in-routing
Open

fix(routing): prevent dynamic static-suffix routes from shadowing static routes#6
JaredStowell wants to merge 1 commit intomainfrom
codex/fix-route-precedence-regression-in-routing

Conversation

@JaredStowell
Copy link
Owner

Motivation

  • A recent route-precedence refactor allowed dynamic routes that include static infix/suffix segments (e.g. /:id/settings) to score lower than fully static routes (e.g. /users/settings), causing dynamic routes to be tried before static ones and shadow them.
  • The intended semantics are that purely static routes always take priority over dynamic routes, so the scoring must guarantee static routes remain above dynamic ones.

Description

  • Clamp dynamic route precedence so dynamic patterns cannot score <= 0 by forcing a minimum score of 1 for any route detected as dynamic in routePrecedence.
  • This change preserves the existing relative ordering among dynamic and catch-all routes while ensuring purely static routes (score 0) always outrank any dynamic route.
  • Added a focused regression test that asserts /users/settings (static) outranks /:id/settings (dynamic with static suffix) by importing and exercising routePrecedence in tests/routing.test.ts.
  • Files changed: packages/vinext/src/routing/utils.ts and tests/routing.test.ts.

Testing

  • Ran the targeted routing test suite with pnpm vitest run tests/routing.test.ts, which completed and passed the routing tests including the new regression test.
  • Ran the single-test invocation pnpm vitest run tests/routing.test.ts -t "keeps fully static routes ahead", which also passed and validates the specific case.
  • The fix is minimal and localized to the precedence calculation so it preserves existing behavior observed by the routing tests.

Codex Task

Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c31379afbe

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +72 to +73
if (isDynamic && score <= 0) {
score = 1;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Avoid collapsing dynamic precedence scores

Clamping every dynamic route with score <= 0 to 1 removes the relative ordering between distinct dynamic patterns that were previously differentiated by infix static specificity. For example, /:a/x/y and /:a/x/:b now both score 1, so the router falls back to lexicographic order and can try /:a/x/:b first, incorrectly shadowing the more specific /:a/x/y on /foo/x/y. This regression is introduced by the new clamp because the prior scoring kept these routes distinct.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant